Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace ancient lazy_static crate with once_cell or const slices #471

Merged
merged 1 commit into from
Jun 13, 2022

Conversation

MarijnS95
Copy link
Contributor

Piggybacking on the motivation in winit: lazy_static! is a macro whereas once_cell achieves the same using generics. Its implementation has also been proposed for inclusion in std, making it easier to switch to a standardized version if/when that happens. The author of that winit PR is making this change to many more crates, slowly turning the scales in favour of once_cell in most dependency trees. Furthermore lazy_static hasn't published any updates for 3 years.

See also the once_cell F.A.Q..

In addition "constant" Vector allocations don't need to be wrapped in a lazy_static! macro call at all but can be replaced with true const slices (or const sized arrays, but those are slightly more tedious to construct).

@codecov-commenter
Copy link

Codecov Report

Merging #471 (f04cedc) into master (02c6775) will increase coverage by 0.16%.
The diff coverage is 100.00%.

❗ Current head f04cedc differs from pull request most recent head 2b216e6. Consider uploading reports for the commit 2b216e6 to get more accurate results

@@            Coverage Diff             @@
##           master     #471      +/-   ##
==========================================
+ Coverage   80.53%   80.69%   +0.16%     
==========================================
  Files          73       74       +1     
  Lines        8475     8549      +74     
==========================================
+ Hits         6825     6899      +74     
  Misses       1650     1650              

Piggybacking on the [motivation in winit]: `lazy_static!` is a macro
whereas `once_cell` achieves the same using generics.  Its
implementation has also been [proposed for inclusion in `std`], making
it easier to switch to a standardized version if/when that happens.  The
author of that winit PR is making this change to many more crates,
slowly turning the scales in favour of `once_cell` in most dependency
trees.  Furthermore `lazy_static` hasn't published any updates for 3
years.

See also [the `once_cell` F.A.Q.].

In addition "constant" `Vec`tor allocations don't need to be wrapped in
a `lazy_static!` macro call at all but can be replaced with true `const`
slices (or `const` sized arrays, but those are slightly more tedious to
construct).

[motivation in winit]: rust-windowing/winit#2313
[proposed for inclusion in `std`]: rust-lang/rust#74465
[the `once_cell` F.A.Q.]: https://docs.rs/once_cell/latest/once_cell/#faq
Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

I have been following this for quite a while, but rather waiting for it to stabilize in std.

"anyhow::",
"log::",
];
const WELL_KNOWN_SYS_MODULES: &[&str] = &[
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! this could have been done independently anyway. No reason for these being Vecs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, exactly!

@Swatinem Swatinem merged commit 4b04b44 into getsentry:master Jun 13, 2022
@MarijnS95
Copy link
Contributor Author

I have been following this for quite a while, but rather waiting for it to stabilize in std.

Yeah, not sure when it lands but this brings us a step closer at least :)

@MarijnS95 MarijnS95 deleted the once-cell branch June 13, 2022 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants